Website: Add error report modal #1102
Conversation
…/wordpress-playground into add/error-report-modal
|
@adamziel What do you think about the copy? |
bgrgicak
force-pushed
the
add/error-report-modal
branch
from
8bd8482 to
978dec8
Compare
bgrgicak
force-pushed
the
add/error-report-modal
branch
from
3835af3 to
978dec8
Compare
packages/playground/website/src/components/error-report-modal/index.tsx
Outdated
Show resolved
Hide resolved
| @@ -33,13 +33,13 @@ function response($ok, $error = null) | |||
| function validate_message($message) | |||
| { | |||
| // Validate description. Description is required | |||
There was a problem hiding this comment.
Wouldn't this be simpler and more reliable if the data was transmitted not as a text blob but as separate POST fields? $_POST['description'], $_POST['logs'] etc? The server could then stitch it together into a slack message.
There was a problem hiding this comment.
I was thinking the same things while working on the validation but ended up overcomplicating it.
There was a problem hiding this comment.
Done, this is definitely much cleaner.
| @@ -5,7 +5,7 @@ AddEncoding x-gzip .gz | |||
| Header unset ETag | |||
| Header set Cache-Control "max-age=0, no-cache, no-store, must-revalidate" | |||
| </FilesMatch> | |||
| <FilesMatch "index\.js|blueprint-schema\.json|wp-cli.phar"> | |||
| <FilesMatch "index\.js|blueprint-schema\.json|logger.php|wp-cli.phar"> | |||
There was a problem hiding this comment.
Is this for Playground integrators?
There was a problem hiding this comment.
This is for localhost, but it's also required if we want to allow an integrator to send error reports.
…index.tsx Co-authored-by: Adam Zielinski <adam@adamziel.com>
| response(false, 'Invalid message'); | ||
| if (isset($_POST['logs'])) { | ||
| if (preg_match('/\[\d{2}-[A-Za-z]{3}-\d{4} \d{2}:\d{2}:\d{2} UTC\](.*)/s', $_POST['logs']) !== 1) { | ||
| response(false, 'Logs do not match the PHP error log format'); |
There was a problem hiding this comment.
I'm a bit skeptical about this check – I think we'll get surprised with some false positives eventually and also this only tests for a single match. What would be a scenario it protects against?
There was a problem hiding this comment.
The only thing it does is ensure the format is correct (if the regex works as expected), but it doesn't protect us.
I will remove it and just check if it's empty.
| @@ -87,6 +88,7 @@ function Main() { | |||
|
|
|||
| return ( | |||
| <PlaygroundContext.Provider value={{ storage }}> | |||
| <ErrorReportModal /> | |||
There was a problem hiding this comment.
Follow-up idea – add a "Report an error" button to trigger the modal manually.
This PR is missing data submission and shouldn't be merged.
What is this PR doing?
When a PHP request fails a modal will load that allows users to report errors to us.
What problem is it solving?
It will help us collect Playground errors that we need to improve stability.
How is the problem addressed?
By allowing users to submit error reports to us.
Testing Instructions
npm run dev